Skip to content

Conversation

@modanesh
Copy link

@modanesh modanesh commented Sep 24, 2025

Description

Provides the task of having the Franka arm push a cube to a randomly selected target position, inspired by https://github.com/Yannyehao/Isaac-Lab-Push

Fixes # (issue)

This PR fixes the issue #3545 and discussion #437.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

A demonstration of the task:

Screenshot 2025-09-24 at 1 28 07 PM

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Sep 24, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Implemented a Franka arm push task where the robot must push a cube to a randomly sampled target position on a table.

Key Implementation Details:

  • Added new Isaac-Push-Cube-Franka-v0 environment with joint position control
  • Created MDP components: observations (object/robot positions), rewards (reaching + goal tracking), and terminations (goal reached/timeout)
  • Configured PPO agents for rsl_rl, skrl, rl_games, and sb3 frameworks
  • Added benchmarking test with reward threshold of 70
  • Updated documentation with new task entry

Issues Found:

  • Multiple naming inconsistencies where "lift" appears instead of "push" in docstrings and comments (likely copy-pasted from lift task)
  • Duplicate copyright header in push_env_cfg.py
  • Physics config sets bounce_threshold_velocity twice with different values (0.2 then 0.01)
  • CHANGELOG version mismatch (0.46.1 vs 0.11.1)
  • Agent configs reference "franka_lift" instead of "franka_push" for experiment names
  • Unused os import in task registration

Confidence Score: 3/5

  • Safe to merge after fixing the critical syntax errors - functionality is sound but naming/documentation issues need correction
  • Score of 3 reflects that while the core implementation is correct and functional, there are multiple syntax errors from copying the lift task template that must be fixed: duplicate copyright headers, incorrect docstrings, wrong experiment names in configs, version mismatch in CHANGELOG, and a logic error with duplicate physics settings. These issues don't break functionality but affect code quality and consistency.
  • Primary attention needed on push_env_cfg.py (duplicate header + physics setting), CHANGELOG (version), and agent configs (experiment names). All docstring/comment fixes are straightforward corrections.

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/push_env_cfg.py 3/5 Core push environment config with duplicate copyright header, duplicate physics setting, and several lift/push naming issues in docstrings
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/mdp/rewards.py 4/5 Reward functions with incorrect comment referencing lifting instead of pushing
source/isaaclab_tasks/docs/CHANGELOG.rst 3/5 CHANGELOG entry with incorrect version number (0.46.1 vs 0.11.1 in extension.toml)
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/config/franka/agents/skrl_ppo_cfg.yaml 4/5 SKRL PPO config with incorrect directory name (franka_lift vs franka_push)
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/config/franka/agents/rl_games_ppo_cfg.yaml 4/5 RL Games PPO config with incorrect experiment name (franka_lift vs franka_push)

Sequence Diagram

sequenceDiagram
    participant User
    participant ManagerBasedRLEnv
    participant PushEnvCfg
    participant Scene
    participant Robot as Franka Robot
    participant Object as Cube
    participant CommandManager
    participant RewardManager
    
    User->>ManagerBasedRLEnv: Initialize Isaac-Push-Cube-Franka-v0
    ManagerBasedRLEnv->>PushEnvCfg: Load configuration
    PushEnvCfg->>Scene: Setup ObjectTableSceneCfg
    Scene->>Robot: Spawn Franka Panda
    Scene->>Object: Spawn Cube on table
    Scene->>Scene: Setup FrameTransformer for end-effector
    
    loop Training Episode
        ManagerBasedRLEnv->>CommandManager: Sample target position
        CommandManager-->>ManagerBasedRLEnv: Return goal pose (x, y, z)
        
        loop Episode Steps
            ManagerBasedRLEnv->>Robot: Get joint positions/velocities
            ManagerBasedRLEnv->>Object: Get cube position
            ManagerBasedRLEnv->>ManagerBasedRLEnv: Compute observations
            ManagerBasedRLEnv-->>User: Return state
            
            User->>ManagerBasedRLEnv: Apply action (joint positions)
            ManagerBasedRLEnv->>Robot: Set joint position targets
            Robot->>Object: Push cube via contact
            
            ManagerBasedRLEnv->>RewardManager: Compute rewards
            RewardManager->>RewardManager: object_ee_distance (reach cube)
            RewardManager->>RewardManager: object_goal_distance (push to target)
            RewardManager->>RewardManager: action_rate & joint_vel penalties
            RewardManager-->>ManagerBasedRLEnv: Total reward
            
            ManagerBasedRLEnv->>ManagerBasedRLEnv: Check terminations
            alt Object reached goal
                ManagerBasedRLEnv-->>User: Episode done (success)
            else Timeout
                ManagerBasedRLEnv-->>User: Episode done (timeout)
            else Continue
                ManagerBasedRLEnv-->>User: Continue episode
            end
        end
        
        ManagerBasedRLEnv->>Scene: Reset environment
        Scene->>Object: Randomize cube position
        Scene->>Robot: Reset to default pose
    end
Loading

41 files reviewed, 14 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 1 to 9
# Copyright (c) 2022-2025, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause

# Copyright (c) 2022-2024, The Isaac Lab Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: duplicate copyright header

Suggested change
# Copyright (c) 2022-2025, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) 2022-2024, The Isaac Lab Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) 2022-2025, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause

Comment on lines 217 to 218
self.sim.physx.bounce_threshold_velocity = 0.2
self.sim.physx.bounce_threshold_velocity = 0.01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: bounce_threshold_velocity set twice with different values (0.2 then 0.01). The second assignment overwrites the first.

Suggested change
self.sim.physx.bounce_threshold_velocity = 0.2
self.sim.physx.bounce_threshold_velocity = 0.01
self.sim.physx.bounce_threshold_velocity = 0.01

#
# SPDX-License-Identifier: BSD-3-Clause
import gymnasium as gym
import os
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: os imported but never used

Suggested change
import os

#
# SPDX-License-Identifier: BSD-3-Clause

"""Configurations for the object lift environments."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: docstring says "lift" but should say "push"

Suggested change
"""Configurations for the object lift environments."""
"""Configurations for the object push environments."""

#
# SPDX-License-Identifier: BSD-3-Clause

"""Configurations for the object lift environments."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: docstring says "lift" but should say "push"

Suggested change
"""Configurations for the object lift environments."""
"""Configurations for the object push environments."""

time_limit_bootstrap: False
# logging and checkpoint
experiment:
directory: "franka_lift"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: directory name is "franka_lift" but should be "franka_push"

Suggested change
directory: "franka_lift"
directory: "franka_push"

load_path: '' # path to the checkpoint to load

config:
name: franka_lift
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: experiment name is "franka_lift" but should be "franka_push"

Suggested change
name: franka_lift
name: franka_push

des_pos_w, _ = combine_frame_transforms(robot.data.root_pos_w, robot.data.root_quat_w, des_pos_b)
# distance of the end-effector to the object: (num_envs,)
distance = torch.norm(des_pos_w - object.data.root_pos_w, dim=1)
# rewarded if the object is lifted above the threshold
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: comment says "rewarded if the object is lifted above the threshold" but this is a push task, not a lift task

Suggested change
# rewarded if the object is lifted above the threshold
# reward based on distance to goal position

# distance of the end-effector to the object: (num_envs,)
distance = torch.norm(des_pos_w - object.data.root_pos_w[:, :3], dim=1)

# rewarded if the object is lifted above the threshold
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: comment says "rewarded if the object is lifted above the threshold" but should describe the termination condition

Suggested change
# rewarded if the object is lifted above the threshold
# terminate if object reached goal position

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR adds a new manipulation task for pushing a cube with the Franka robot arm to a randomly selected target position, addressing issue #3545.

Major Changes:

  • New push environment with table-based scene configuration (push_env_cfg.py)
  • Custom MDP functions: rewards based on EE-to-object and object-to-goal distances, termination on goal reaching
  • Franka-specific configuration with joint position control and binary gripper actions
  • PPO agent configurations for rsl_rl, skrl, rl_games, and sb3 frameworks
  • Robomimic BC/BCQ configurations for imitation learning support

Issues Found:
All syntax issues mentioned in previous comments have been properly identified and should be addressed (duplicate docstrings saying "lift" instead of "push", version mismatch in CHANGELOG, duplicate bounce_threshold_velocity assignment)

Confidence Score: 4/5

  • This PR is safe to merge after addressing the syntax/naming issues already flagged in previous comments
  • The implementation is structurally sound with correct reward/termination logic and proper agent configurations. The score is 4 (not 5) due to several syntax issues: duplicate bounce_threshold_velocity assignment (push_env_cfg.py:213), version mismatch between CHANGELOG (0.46.1) and extension.toml (0.11.1), and multiple docstrings/comments incorrectly referencing "lift" instead of "push". All issues are non-critical formatting/naming problems already identified in previous comments.
  • push_env_cfg.py needs the duplicate bounce_threshold_velocity assignment fixed, and CHANGELOG.rst needs version number corrected to match extension.toml

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/push_env_cfg.py 4/5 Core environment configuration for push task - has duplicate bounce_threshold_velocity assignment and version mismatch in CHANGELOG
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/mdp/rewards.py 5/5 Reward functions for pushing task - implementation is correct and consistent with similar environments
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/mdp/terminations.py 5/5 Termination conditions for push task - logic is correct for goal-reaching detection
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/config/franka/joint_pos_env_cfg.py 5/5 Franka-specific environment configuration - properly extends base push config
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/push/config/franka/agents/rsl_rl_ppo_cfg.py 5/5 RSL-RL PPO agent configuration - reasonable hyperparameters for push task

Sequence Diagram

sequenceDiagram
    participant Env as Push Environment
    participant Robot as Franka Robot
    participant Cube as Cube Object
    participant Goal as Goal Position
    participant Agent as RL Agent

    Note over Env: Episode Start
    Env->>Cube: Reset position (random x: -0.1 to 0.1, y: -0.25 to 0.25)
    Env->>Goal: Sample goal position (x: 0.4 to 0.6, y: -0.25 to 0.25)
    Env->>Agent: Observe (joint_pos, joint_vel, object_pos, goal_pos, actions)
    
    loop Every timestep (50Hz with decimation=2)
        Agent->>Env: Action (arm_joint_positions, gripper_open/close)
        Env->>Robot: Apply joint position commands
        Robot->>Cube: Push/contact cube
        Cube->>Cube: Move towards goal
        
        Note over Env: Compute Rewards
        Env->>Env: reaching_object (EE to cube distance)
        Env->>Env: object_goal_tracking (cube to goal distance)
        Env->>Env: object_goal_tracking_fine_grained (precise goal tracking)
        Env->>Env: action_rate penalty
        Env->>Env: joint_vel penalty
        
        Env->>Env: Check termination (time_out after 5s)
        Env->>Agent: Observe next state + reward
    end
    
    alt Cube reaches goal
        Note over Env: Success (distance < 0.02m)
    else Timeout
        Note over Env: Episode ends
    end
Loading

9 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Mohamad H. Danesh <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Updates the CHANGELOG to document version 0.11.1, which adds the new Isaac-Push-Cube-Franka-v0 manipulation environment for robotic object pushing tasks.

Key Changes:

  • Adds version 0.11.1 entry with new environment documentation
  • Version number correctly matches extension.toml (0.11.1)

Issues Found:

  • Date in CHANGELOG entry (2025-09-24) predates the actual commit date (2025-11-05)

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk after correcting the date
  • The CHANGELOG update is straightforward and correctly documents the new feature. Version number matches extension.toml. Only issue is incorrect date (uses September instead of November), which is a minor documentation error that doesn't affect functionality.
  • No files require special attention - just fix the date in CHANGELOG.rst

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab_tasks/docs/CHANGELOG.rst 4/5 Adds new version entry with incorrect date (should be 2025-11-05, not 2025-09-24)

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CL as CHANGELOG.rst
    participant ET as extension.toml
    participant Repo as Repository
    
    Dev->>CL: Add version 0.11.1 entry
    Dev->>CL: Document Isaac-Push-Cube-Franka-v0
    Dev->>CL: Set date to 2025-09-24
    Note over CL: Date mismatch:<br/>Entry dated 2025-09-24<br/>Commit dated 2025-11-05
    Dev->>ET: Verify version 0.11.1
    Note over ET,CL: Version numbers match ✓
    Dev->>Repo: Commit changes
    Repo-->>Dev: Commit timestamp: 2025-11-05
Loading

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Mohamad H. Danesh <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR updates the CHANGELOG to document the addition of the new Isaac-Push-Cube-Franka-v0 environment.

  • Updated CHANGELOG.rst with new version entry 0.11.1 dated 2025-11-05
  • Documents the addition of the Franka Push task environment
  • Version number correctly matches extension.toml (no mismatch exists)
  • Date correctly matches the commit timestamp
  • Follows standard changelog formatting conventions

Confidence Score: 5/5

  • This PR is safe to merge - it's a simple documentation update
  • The changes are minimal and straightforward: adding a single changelog entry following the established format. The version number and date are consistent with the extension configuration and commit timestamp. No code logic changes.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab_tasks/docs/CHANGELOG.rst 5/5 Added new version entry for Franka Push environment, correctly formatted and dated

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CL as CHANGELOG.rst
    participant ET as extension.toml
    participant GH as GitHub PR
    
    Dev->>CL: Add version 0.11.1 entry
    Dev->>CL: Document Isaac-Push-Cube-Franka-v0
    Dev->>CL: Set date to 2025-11-05
    Dev->>ET: Update version to 0.11.1
    Dev->>GH: Commit changes (dccdcba6)
    GH->>GH: Validate consistency
    Note over CL,ET: Version and date aligned
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant